Repository navigation
Fix range tombstone GC with ingest-behind resurrecting deleted keys - #15653
Open
Potato-Yao wants to merge 2 commits into
Open
Potato-Yao wants to merge 2 commits into
Potato-Yao wants to merge 2 commits into
Conversation
Summary: Add IngestBehindPreservesRangeDeletions to reproduce meta#15425: Universal compaction drops a range tombstone before an ingest-behind backfill, so historical values in the deleted range become visible again. Compare ingestion with and without prior compaction using both the column-family and legacy DB-wide ingest-behind options. Check newer values, point deletions, range deletions, the exclusive end boundary, and an unaffected key across the existing ingestion parameters. This is a reproducer-only checkpoint. Production behavior is unchanged; the new test intentionally fails until the compaction fix is added. Test Plan: Built external_sst_file_test in debug mode with AUTO_CLEAN=1 DISABLE_WARNING_AS_ERROR=1 make -j6 external_sst_file_test. The warning override was needed for existing GCC 16 BlobDB warnings. Ran *IngestBehindPreservesRangeDeletions* and repeated it 100 times: all four parameter variants fail only on the two range-deleted keys after compaction. The no-compaction controls and other assertions pass. Formatting and git diff --check pass. Full-suite and coerce-mode validation are deferred until the fix stage.
…ed, which could cause a subsequent historical backfill to resurrect range-deleted keys.
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues block the change to preserve range tombstones when ingest-behind is enabled.
No open issues 🎉
What changed in this PR
Fixes #15425 by preserving range tombstones during compaction when ingest-behind is enabled. Previously, compaction could drop these tombstones, allowing a later backfill to resurrect range-deleted keys.
Changes:
- Prevents obsolete range-tombstone removal when either per-column-family or DB-wide ingest-behind is enabled.
- Adds regression coverage for both option styles, with and without compaction, checking deleted keys, newer values, and range boundaries.
No public API or persisted-format changes are introduced; tombstone-removal behavior remains unchanged when ingest-behind is disabled.
Validation: The PR reports all four parameter variants and 400 repeated executions passing, with the regression reproduced when the fix is removed. Full-suite, forced-context-switch, and non-Linux validation remain outstanding.
| File | Description |
|---|---|
db/external_sst_file_test.cc |
Adds ingest-behind regression coverage for range-deletion preservation. |
db/compaction/compaction_outputs.cc |
Guards range-tombstone removal with both ingest-behind options. |
Review effort: Balanced
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix #15425. Compaction could garbage-collect a range tombstone at the bottommost level even when ingest-behind was enabled. Because a later ingest-behind backfill inserts files below all existing data, the dropped tombstone no longer masked the backfilled values, so range-deleted keys became visible again.
Previous behavior and root cause
In
CompactionOutputs::AddRangeDels(), a range tombstone was considered obsolete and dropped when:tombstone.seq_ <= earliest_snapshot, andfull_history_ts_low.That is valid only while no older data can later appear beneath the tombstone. Ingest-behind breaks that assumption by placing files with older sequence numbers below all existing data. Point tombstones already had the analogous protection in
CompactionIterator::ProcessDeletion()(!compaction_->allow_ingest_behind()), but the range-tombstone path did not.Reproducer:
Put("a", "new"),Delete("b"),DeleteRange("c", "d"), flush, force a bottommost compaction, then ingest-behind{a,b,c,c2,d,e} = "old". Before the fix,candc2read as"old"instead ofNOT_FOUND.Implementation
db/compaction/compaction_outputs.cc:consider_dropinCompactionOutputs::AddRangeDels()now additionally requires bothimmutable_options().cf_allow_ingest_behindandimmutable_options().allow_ingest_behindto be false. No API, on-disk format, or MANIFEST change.Test Plan
Added
ExternalSSTFileTest.IngestBehindPreservesRangeDeletionsindb/external_sst_file_test.cc. The test runs under four ingestion configurations (checksum verification and two-phase ingest each on/off) and, inside each, loops over:cf_allow_ingest_behindversus DB-wideallow_ingest_behindIt checks that a newer value survives (
a), a point deletion stays deleted (b), range-deleted keys stay deleted (c,c2), the exclusive end boundary is unaffected (d), and a key outside the range is backfilled (e).Validation performed:
compact_before_ingest = true, under both option styles, withGet("c")andGet("c2")returning"old". The no-compaction controls pass.-r100) pass with no failures.Commands:
DISABLE_WARNING_AS_ERROR=1 build_tools/rockstest.sh external_sst_file_test \ --gtest_filter='*IngestBehindPreservesRangeDeletions*' DISABLE_WARNING_AS_ERROR=1 build_tools/rocksptest.sh external_sst_file_test \ -r100 --gtest_filter='*IngestBehindPreservesRangeDeletions*'DISABLE_WARNING_AS_ERROR=1is needed only because of pre-existing GCC 16 BlobDB warnings. Not yet run: fullmake check,COERCE_CONTEXT_SWITCH=1stress mode, and non-Linux platforms.